Skip to content

use_air() also turns on "format on save" for qmd + other updates - #2136

Merged
jennybc merged 4 commits into
mainfrom
use-air-quarto-too
Jul 10, 2025
Merged

use_air() also turns on "format on save" for qmd + other updates#2136
jennybc merged 4 commits into
mainfrom
use-air-quarto-too

Conversation

@jennybc

@jennybc jennybc commented Jul 9, 2025

Copy link
Copy Markdown
Member

Closes #2135

@jennybc
jennybc requested review from DavisVaughan and lionel- July 9, 2025 20:27
Comment thread R/air.R Outdated
Comment on lines +46 to +49
#' - For Positron, install the [OpenVSX
#' Extension](https://open-vsx.org/extension/posit/air-vscode).
#' - For Positron, the [Air extension](https://open-vsx.org/extension/posit/air-vscode)
#' is installed by default and that already includes the Air binary. A typical
#' Positron user does not need to do anything about installing Air.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we needed this important update anyway!

@DavisVaughan DavisVaughan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a question about the ^[\.]?air\.toml$ change

Comment thread R/air.R Outdated
Comment thread R/air.R
air_toml_regex <- function() {
# Pre-escaped regex allowing both `air.toml` and `.air.toml`
"^[\\.]?air\\.toml$"
"^[.]?air[.]toml$"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the \\. was purposeful. i.e. we have ^\.vscode$ in our Rbuildignore along with many other \. usage.

Unless the [] does something special, I think we should end up with [\.] as well, otherwise doesn't a lone . mean "anything" or something like that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot tells me they are functionally the same, but \. is probably more common for escaping a literal ., and that seems to align with our existing conventions in .Rbuildignore. Thoughts?

Screenshot 2025-07-10 at 8 41 30鈥疉M

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[] is commonly used as a less heavy quote syntax for single characters. I like it as it's easier to read.

@jennybc jennybc Jul 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's perhaps a really small hill to die on, but, yes, I am a big fan of using [] instead of escaping, when possible. (But yes, you'll see some escaping in usethis because history.) I really don't care deeply about this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The [] is fine

Comment thread R/air.R
Comment on lines -145 to -158
settings <- jsonlite::read_json(path)
settings_r <- settings[["[r]"]]

if (is.null(settings_r)) {
# Mock it
settings_r <- set_names(list())
}

# Set these regardless of their previous values. Assume that calling
# `use_air()` is an explicit request to opt in to these settings.
settings_r[["editor.formatOnSave"]] <- TRUE
settings_r[["editor.defaultFormatter"]] <- "Posit.air-vscode"

settings[["[r]"]] <- settings_r

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a little worried modifyList() would overwrite an existing [r] or [quarto] setting that wasn't formatOnSave or defaultFormatter, but I see your new test with

    "[quarto]" = list(
      "editor.wordWrap" = "wordWrapColumn"
    )

And it looks like it preserves that, so that's a win for modifyList() I guess 馃憤

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really is designed for exactly this.

Comment thread R/air.R
air_toml_regex <- function() {
# Pre-escaped regex allowing both `air.toml` and `.air.toml`
"^[\\.]?air\\.toml$"
"^[.]?air[.]toml$"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[] is commonly used as a less heavy quote syntax for single characters. I like it as it's easier to read.

Co-authored-by: Davis Vaughan <davis@rstudio.com>
@jennybc
jennybc merged commit e6501b9 into main Jul 10, 2025
15 checks passed
@jennybc
jennybc deleted the use-air-quarto-too branch July 10, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

use_air() should set formatOnSave for Quarto too

3 participants